-
Notifications
You must be signed in to change notification settings - Fork 110
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Next version! #732
Next version! #732
Conversation
Teal Playground URL: https://732--teal-playground-preview.netlify.app |
From a quick glance I like the api changes and interfaces are a huge boon. Writing up some quick and dirty luv definitions (which is full of inheritance/interfaces) seems to be working well. I see I do need to get back into the swing of working on teal related stuff, but I definitely fell into the got-a-real-job-and-now-have-less-time-to-work-on-hobby-projects trap that happens so often with open source. All in all, looks great to me! |
Same as @euclidianAce, real life (work + new kid) means I have little time for OSS but the changes I see in this branch are a lot of things I thought would be great for Teal. Once it is a bit more stable I'll try it on my existing code bases in Teal. |
@bjornbm tests and documentation are still pretty much missing, because I wanted to iterate as quickly as possible to experiment with the usability of these features... but once the dust settles we definitely need them! |
@euclidianAce @catwell Thanks a lot for the feedback!! And, yeah, I totally get that sentiment — over the years I've accepted that my work on FOSS projects happens in bursts. When I find that I get a window of availability and motivation to work on this stuff, I try to get the most out of it. :) For Teal, having a quiet time for the project since the last burst was a good thing as it allowed me to set priorities: seeing what are the gaps that people most often try to solve, etc. I think it's clear by now that the top two things missing in current Teal are interfaces and better |
Also, speaking of writeups, it's funny that CI is failing because the |
Update: it took me a big reorganization in d885f90 to get CI green. This is because the size of tl.tl started hitting size limits in both Lua 5.1 (number of upvalues) and Lua 5.4 (number of locals). The goal of the changes in this latest commit is to be able to finally break tl.tl into multiple files, but I wanted to do the reorg first to produce a sensible diff (which would let me more easily track the changes). The commit description includes more details on the internal changes. |
Regarding "making the type system more nominal", here's one interesting example: https://toot.kottman.xyz/@michal/111860362351389256
https://mastodon.social/@hisham_hm/111861339267098398
This checks without errors in the local type Code = integer
local function get_code(): Code
end
local type Age = integer
local function get_age(): Age
end
local x: integer = get_code()
x = get_age()
local c: Code = get_code()
c = get_age() -- ERROR: in assignment: Age is not a Code
local a: Age = get_age()
a = get_code() -- ERROR: in assignment: Code is not a Age |
I played a bit with the
but not this:
It results in:
Is that expected or do you intend to support it? (Slightly related: can you release your FOSDEM slides? I know the video is probably broken, which is too bad, but the slides at least would be useful.) |
@catwell thanks for the test case! I haven't really worked on the relationship between generics and interfaces/
I intended |
The video was not broken \o/ |
How are |
25cea1d
to
9cd464e
Compare
88918b2
to
e69d91b
Compare
f9ceea1
to
fbac8f3
Compare
f462b62
to
e297db7
Compare
I can confirm tls working well with tl next with just some minor fixes. Would it be possible to get preview version of tl published to luarocks? Then I can do the same with tls. Or alternatively I can just wait until it merges and use an official release. Btw - Really appreciate the arity and interface features. After upgrading it's clear these will be very useful in our codebase. Upgrading was pretty straightforward for us - even caught some potential runtime bugs after enabling the arity check |
You can do
I think that should be fine. It should be a matter of days!
That is great to hear! Thank you so much for the positive feedback, it means a lot!! |
@pdesaulniers I'm assuming it's a +1 from vscode-teal as well, since you've added the extra syntax highlighting in 0.10.0, and @V1K1NGbg has used vscode-teal 0.10.0 happily with tl |
@hishamhm Autocompletion behaves a bit strangely right now, but I haven't had the time to debug this further... (Also, I'm not sure if these issues are actually specific to the I can at least reproduce this specific case:
It seems like the two local variables |
2021 does, there are still a few errors on 2020 :) In particular it does not like when I call this with a single argument (e.g in day13 part1):
In day20 there is a different error probably related to the "empty table" issue:
Line 238 is |
I guess my |
@pdesaulniers Interesting. This works:
...so it looks like it is ending the scope "too early"? Might be a simple fix. BTW, looks like it also happens on |
This should also avoid inferring types as `{} | {}`.
😬 All of 2020 and 2021 should be passing with |
@pdesaulniers That test case should be now fixed! It seemed to be specific to if-blocks |
@hishamhm I think I found a point.tl: local record Point
x: number
y: number
end
function Point:init(x: number, y: number)
self.x = x
self.y = y
end
Before (master branch): "4": {
"fields": {
"init": 2,
"x": 1,
"y": 1
},
"file": "point.tl",
"str": "type Point",
"t": 131080,
"x": 1,
"y": 1
},
"5": {
"fields": {
"init": 2,
"x": 1,
"y": 1
},
"file": "point.tl",
"str": "Point",
"t": 131080,
"x": 1,
"y": 1
}, After (next branch): "9": {
"fields": {
"x": 5,
"y": 5
},
"file": "point.tl",
"str": "record",
"t": 131080,
"x": 1,
"y": 1
},
"10": {
"fields": {
"x": 5,
"y": 5
},
"file": "point.tl",
"str": "type record",
"t": 131080,
"x": 1,
"y": 1
}, 'init' is absent in the fields list. |
Also, strip typedecls from type report, and represent records in the "str" field by their declared name. Thanks @pdesaulniers for the report!
@pdesaulniers it should be fixed now! the construction of the type report now happens as part of the type checking pass. Records with functions are (IIRC!) the only types that "grow" over time as one reads through a file, and their type definitions were not being updated in the report (because the typeinfo entries are memoized). 6117c8c should fix this, testcase included. |
Great, seems OK now! I think I noticed another local record Operator
operator: string
end
local record Node
node1: Node
operator: Operator
end
local function node_is_require_call(n: Node): string
if n.operator and n.operator.operator == "." then
return node_is_require_call(n.node1)
end
end $ tl types test.tl > test.json In the master branch, there was a "ref" field which would lead to the record declaration (I simplified the output to make it a bit easier to read): "by_pos": {
"test.tl": {
"11": {
"9": 5,
"24": 5,
}
}
},
"types": {
"5": {
"file": "test.tl",
"ref": 6, /* HERE */
"str": "Operator",
"t": 268435456,
"x": 15,
"y": 7
},
"6": {
"fields": {
"operator": 1
},
"file": "test.tl",
"str": "type Operator",
"t": 131080,
"x": 1,
"y": 1
},
} In the next branch, this 'ref' field is missing (see type 12): "by_pos": {
"test.tl": {
"11": {
"9": 12,
"24": 12,
},
}
},
"types": {
"9": {
"fields": {
"operator": 7
},
"file": "test.tl",
"str": "Operator",
"t": 131080,
"x": 1,
"y": 1
},
"12": { /* MISSING REF HERE */
"file": "test.tl",
"str": "Operator",
"t": 268435456,
"x": 15,
"y": 7
}
} However, in the next branch, if I reverse the order of the fields in the local record Node
operator: Operator
node1: Node
end "9": {
"fields": {
"operator": 7
},
"file": "test.tl",
"str": "Operator",
"t": 131080,
"x": 1,
"y": 1
},
"10": {
"file": "test.tl",
"ref": 9, /* HERE */
"str": "Operator",
"t": 268435456,
"x": 15,
"y": 6
}, |
See #732 (comment) > There is one remaining tricky error triggering in the day12 for which I > already found a workaround (but which would merit a nicer solution since this > uncovered the fact that my empty-table inference does not backpropagate > correctly in the case of map[c1][c2] = true). I'll try to code a solution > quickly, otherwise I'll document the edge case in the testsuite and go with > the workaround. This is the workaround, and the documentation of the edge case is in the test case included in this commit.
Checklist for next version release:
master
are compatible (vscode-teal, Cyan, Defold, etc.)next
-only)